Honor allow_half_open when a client aborts early - #13560
Conversation
Operators who set proxy.config.http.allow_half_open to 0 expect a client abort to tear down the transaction, including the connection to the origin server. Since 10.1, that no longer happens when the client goes away before the origin has sent its response header: ATS holds the origin connection open until the response arrives, so a slow origin combined with impatient clients can accumulate connections until max_requests_in or connections_throttle is reached. The path that keeps the state machine alive in that window exists so that background fill works for clients whose transport cannot half close a connection, such as TLS and HTTP/2. It was reachable for two unrelated reasons, though, since ProxyTransaction::allow_half_open() reports false both for those transports and for an operator who disabled half open connections outright. This patch distinguishes the two by also requiring that half open connections be configured before keeping the transaction alive for a background fetch. Background fill continues to work for TLS and HTTP/2 clients under the default configuration, while disabling half open connections once again aborts the transaction and drops the origin connection. The accompanying autest exercises both outcomes with an origin that reports whether the proxy closed the connection, and the documentation for allow_half_open now describes the interaction. Fixes: apache#13549
|
[approve ci autest 1] |
Labeeb2339
left a comment
Labeeb2339
left a comment
There was a problem hiding this comment.
I reviewed the state-machine guard, the updated record documentation, and the autest. Requiring allow_half_open > 0 preserves the existing background-fill path for transports that cannot half close while making an explicit operator setting of 0 take the abort/close branch. The test matrix covers plain HTTP with half-open disabled and TLS with both disabled and enabled, using an origin-side connection observer rather than inferring behavior from the client timeout. The documented interaction matches the code, and all reported AuTest/platform checks are green. I found no additional issue in this diff.
|
|
||
| origin = tr.Processes.Process( | ||
| f'origin_{self._name}', f'python3 {ORIGIN_SCRIPT} {self._origin_port} --delay {ORIGIN_DELAY_SECONDS}') | ||
| origin.Ready = When.PortOpen(self._origin_port) |
cmcfarlen
left a comment
cmcfarlen
left a comment
There was a problem hiding this comment.
Claude suggested adding h2 tests, but the codepath should be the same, so I'll leave that up to you to add.
Operators who set proxy.config.http.allow_half_open to 0 expect a
client abort to tear down the transaction, including the connection to
the origin server. Since 10.1, that no longer happens when the client
goes away before the origin has sent its response header: ATS holds the
origin connection open until the response arrives, so a slow origin
combined with impatient clients can accumulate connections until
max_requests_in or connections_throttle is reached.
The path that keeps the state machine alive in that window exists so
that background fill works for clients whose transport cannot half
close a connection, such as TLS and HTTP/2. It was reachable for two
unrelated reasons, though, since ProxyTransaction::allow_half_open()
reports false both for those transports and for an operator who
disabled half open connections outright.
This patch distinguishes the two by also requiring that half open
connections be configured before keeping the transaction alive for a
background fetch. Background fill continues to work for TLS and HTTP/2
clients under the default configuration, while disabling half open
connections once again aborts the transaction and drops the origin
connection. The accompanying autest exercises both outcomes with an
origin that reports whether the proxy closed the connection, and the
documentation for allow_half_open now describes the interaction.
Fixes: #13549